Security fixes for 1.1.1 - #9

Two defects found by exercising the new code directly rather than through the
unit suite, which passed with both present.

maskUrlUserinfo now returns an already-masked URL unchanged. Delegating each
candidate to it made maskUrlUserinfoInText non-idempotent: re-masking
`https://***:***@host` produces a byte-identical string, which the fail-closed
check reads as "credentials the regex could not isolate" and replaces with
[URL_WITH_CREDENTIALS_REDACTED]. That path is now reachable because the debug
redactor masks centrally, so a value can arrive here twice. It failed safe but
destroyed the diagnostic. Nothing leaks either way: the userinfo is literally
`***`.

The streamed-content cap no longer splits a surrogate pair. Slicing at 1MB can
cut between the halves of an astral character and leave a lone surrogate in the
accumulated response.

Four of six findings accepted. Three were regressions this branch introduced,
and the suite passed with all of them present.

format.ts, credential leak (High): tokenizing URL candidates on whitespace
swallowed closing delimiters and following URLs, so `<https://u:p@host>`,
`[https://u:p@host]`, and comma-adjacent URLs passed through unmasked. All were
masked before this branch. Restored the original whitespace-and-/?#-bounded
pattern as the primary pass, which stops at a delimiter or the next scheme, and
narrowed the fail-closed sweep to candidates that actually carry tab/CR/LF,
which is the only case the WHATWG parser sees and that pattern cannot.

error-sanitizer.ts, credential leak (High): the new 16KB cap truncated before
redaction, so a URL whose `@` fell past the boundary no longer matched the
credential pattern and its userinfo was emitted verbatim. Probed at four
offsets, all leaked. Truncation now ends on a whitespace boundary, so every
token that survives is whole; a single token longer than the cap is dropped
rather than half-emitted.

jobs/watch.ts (Medium): Dashboard-controlled batch result labels render through
safeString(), which preserves CR/LF/tab, so a result named "site\n✓ Job
completed" forges a status line. Same class as the formatKeyValue leak found
during live-verify, and the same fix.

login.ts (High, partially accepted): login reads MAINWP_APP_PASSWORD directly,
outside the new binding. It takes its destination from --url and never from
profiles.json, so the tampered-file attack the binding exists to stop does not
reach it, and requiring the operator to repeat the URL would break the
documented non-interactive flow. But a declared destination should still be
honoured, so login now refuses when MAINWP_DASHBOARD_URL disagrees with --url
while allowing it to be absent. The binding check moved to a shared exported
function so both callers use one implementation.

Findings 4 and 5 (unbounded provider tool-argument accumulation, unbounded
non-streaming response body) are pushed back with reasoning in
.mwpdev/reviews/REVIEW_DECISIONS.md: both are pre-existing provider-boundary
surfaces, F18 is named out of scope by the 1.1.1 handoff, and both need the
same budget-threading change that does not belong in this PR.

Verified: typecheck, lint, 986 unit tests, 106 process tests, build, and each
of Codex's probes re-run against the fixes. Live-verified the binding still
refuses on the authenticated path and that login accepts an absent declaration
but refuses a mismatched one.

Both reviewers landed on the same conclusion about maskUrlUserinfoInText, from
different angles, so it is now a linear scanner instead of a fourth regex.

The masking hole: `https:\n//user:pass@host` and `https:user:pass@host` are both
credentialed URLs to the WHATWG parser, which discards tab/CR/LF anywhere
(including inside `://`) and gives special schemes an authority without `//`.
Neither pattern could see either form, so the credential printed in cleartext.
Codex also measured the surrounding pattern as quadratic, 947ms at 32k
characters and no completion at 2M, which matters because this now runs on every
debug value. The scanner walks a copy with those characters removed, maps
offsets back so only the matching span is rewritten, and resumes past the last
`@` in each authority so adjacent URLs are still found. 2M characters complete in
~66ms. Brackets, adjacency, `@` in passwords, idempotency and mailto are pinned
by tests.

Truncation could still expose a credential: tab, CR and LF are not safe token
boundaries, because the parser ignores them. Cutting on the newline inside
`https://user:secret\n...@host` kept `https://user:secret`, which no longer
matched the credential pattern. Boundaries now exclude those three characters.

login now requires MAINWP_DASHBOARD_URL, reversing the exemption from round 1.
The argument that `--url` already names the destination does not survive the CI
case Codex put: the password is a protected secret, command arguments usually
are not, so anyone who can edit the pipeline can redirect it without touching
the secret. Binding is worth having only if nothing skips it, so the
requireDeclaration escape hatch is gone. Docs and the process harness updated;
the harness now resolves login's URL from --url rather than the active profile.

Also fixed: an async validator's promise is adopted before the schema guard
throws, so its rejection cannot become the unhandled crash that guard exists to
prevent; the stream cap uses >= so a surrogate pair landing exactly on the
boundary is trimmed, and truncated content reports finishReason 'length' rather
than passing as a complete answer; sanitizeMultiLine collapses tabs, which a
hostile description could use to fake columns; and a malformed profile URL fails
closed as an AuthError instead of a raw TypeError.

Two CodeRabbit findings pushed back in REVIEW_DECISIONS.md: the adjacent-URL
finding describes the superseded implementation it saw in the branch diff, and
the request for hostile-URL cases in the profile command tests duplicates
coverage that belongs to the masker. One matched the existing deferral for
provider tool-argument accumulation.

Verified: typecheck, lint, 987 unit tests, 106 process tests, build,
git diff --check, every probe from both reviewers re-run against the fixes, and
live-verified that login refuses an undeclared or mismatched destination and
succeeds on a matching one.

…URL var)

Making MAINWP_DASHBOARD_URL mandatory broke every documented GitHub Actions
workflow. The guides already store the Dashboard URL as secrets.DASHBOARD_URL,
but nothing exposed it under the name the CLI now requires, so those pipelines
would have failed at their first authenticated command. CodeRabbit caught the
mapping; auditing the rest of the docs for the same omission found four more
places.

Both env: blocks in monthly-batch-updates.md and both in
plugin-deployment-verification.md now set MAINWP_DASHBOARD_URL from the existing
secret, so the "three secrets" setup steps stay correct, and the env:
explanation in each says why it is needed. getting-started.md sets both
variables in its shell and PowerShell examples, and acceptance-testing.md
records that the runner supplies it.

One pushback in REVIEW_DECISIONS.md: the request to drop
`export MAINWP_APP_PASSWORD='xxxx ...'` from the examples. The value is a
placeholder, the reader needs to see how the variable is set, and interactive
prompting, keychain storage, and the --password process-list warning already
cover the concern where it belongs.

Verified: typecheck, lint, 987 tests, git diff --check.

The scanner from round 2 had two credential leaks and had reintroduced the
quadratic behaviour it was written to remove. Replaced its per-colon loop with a
single forward pass that carries authority state.

Leaks: `https:/u:p@h/x` passed through unchanged, because the scanner accepted
only exactly `//` or no slashes at all, while the parser tolerates any run of
slashes after a special scheme. And in `https://safe,https://u:p@h/x` the second
URL escaped entirely: the first authority ran through `,https:` to the slash,
found no `@`, and the scan resumed past the second scheme's colon. A safe URL
sitting next to a credentialed one hid it.

Complexity: `scan.lastIndexOf('@', end - 1)` searched backward from every
authority to a distant `@`, which is quadratic when many short authorities
follow one. Measured 130ms, 512ms and 2034ms over 8k/16k/32k authorities. The
forward pass tracks the last `@` as it goes, so 200k now takes 55ms.

The same pass fixes two false positives: a backslash ends the authority for
special schemes, so the `@` in `https://h\path@x` belongs to the path, and
`file:` is no longer treated as credential-bearing, so `file:u:p@h/x` is left
as the local path it is.

Elsewhere: the stream cap tracks whether it has been reached rather than
inferring it from length, because trimming an orphaned high surrogate drops
back under the cap and the next chunk was then appending its unpaired low half;
the schema guard now contains a hostile thenable whose `then` throws on access
or hands back a rejected promise, neither of which may replace the schema
error; and error truncation scans backward for its boundary instead of using a
trailing-anchored pattern, which discarded an entire message when the tail was a
long run of tabs.

Round 3 traced the credential binding end to end and found no bug there.

One finding pushed back in REVIEW_DECISIONS.md: protocol-relative references.
There is no base to resolve against in error text, nothing here produces
scheme-less URLs, and matching bare `//` would rewrite commented-out code.

Verified: typecheck, lint, 993 unit tests, 106 process tests, build,
git diff --check, and every probe from the round re-run against the fixes,
including the exact chunk sequence for the surrogate case.

Found while probing the forward-pass scanner: an authority ended only at
`/ ? # \` or whitespace, so in `{"url":"https://h.test","user":"a@b"}` it ran
from the URL through the rest of the object to the address's `@` and rewrote
everything between them as userinfo. Dashboard error bodies are JSON, so this
corrupted exactly the diagnostics the masker is supposed to leave readable.

Authorities now also end at the characters a URI cannot contain unencoded:
`" < > ` { } | ^`. Sub-delimiters are deliberately not in that set. They are
legal in userinfo, so ending an authority on one would cut `pa,ss@host` short
of its `@` and turn a display bug into a credential leak. Both directions are
pinned by tests.

Verified: typecheck, lint, 995 tests, plus the full probe set (15 credential
forms masked or failed closed, 11 non-credential inputs unchanged, idempotent,
linear at 1M-character scale).

Three more defects in the forward-pass scanner, all in how an authority opens
and how much text a match replaces.

`http:` appearing inside a password was read as a new URL starting, which closed
the authority it belonged to: `https://u:http:p@h/x` masked only the tail and
left `u:http:` in the output. A special scheme with no slashes now opens an
authority only when none is already open, so a scheme-like substring inside
userinfo no longer splits the URL it is part of.

Any scheme followed by a single slash was treated as having an authority, but
the parser gives that behaviour only to special schemes. `custom:/u:p@h/x` and
`file:/u:p@h/x` are paths and were being rewritten. Non-special schemes now need
a real `//`.

Replacement started at the scheme, so a span whose offsets had shifted took the
text in front of it too: `PRE\nhttps://u:p@h/x` lost `PRE` along with the
credential. Only the userinfo is replaced now, and "obscured" is judged on that
region alone. Obscured cases still fail closed but keep their surroundings and
host, which is what the function always claimed to do:
`x https://u:se\ncret@h/x y` now yields
`x https://[URL_WITH_CREDENTIALS_REDACTED]h/x y`.

The fourth finding, an authority running through JSON to a later `@`, was
already fixed in 94ad3f5 while this round was in flight.

Round 4 found no bug in truncateAtTokenBoundary, the stream cap, or the schema
guard, and measured the scanner linear across four worst-case shapes up to two
million characters.

Verified: typecheck, lint, 999 unit tests, 106 process tests, build,
git diff --check, and the full probe set re-run (15 credential forms masked or
failed closed, 11 non-credential inputs unchanged, idempotent, linear).

Found while probing span edges: `https://@host` and forms the parser empties by
dropping control characters were rewritten to `***:***@`, which claims a
credential had been there when the parser reports none. Emitting a span now
requires the userinfo to be non-empty. A bare username is still a credential and
is still masked.

Round 5 found three parser mismatches, one of them a leak introduced by my own
previous fix. The function now asks the URL parser what is credentialed instead
of deciding from a character table.

The leak: `" < > ` { } | ^` were added as authority terminators to stop a URL
running through surrounding JSON. RFC 3986 forbids those characters, which is
true and irrelevant — the parser that actually runs follows WHATWG, which
percent-encodes them inside userinfo. So `https://user:pa"ss@host` really does
carry a password, and ending the authority at the quote walked past its `@` and
printed the credential in full. Eight characters, eight leaks.

Second, a scheme longer than the 32-character backward walk went unrecognised
whenever the character at the boundary was a digit, so `a1111…://u:p@h` was
never masked. The scan now tracks the start of the current scheme-legal run as
it moves forward, which recognises a scheme of any length in constant time and
removes the limit rather than raising it.

Third, backslashes counted as slashes for every scheme, so `custom:\\u:p@h`
was rewritten although it is a path. Only special schemes treat them that way.

Deciding by parser needed two guards to stay useful: the candidate ends where
the host ends, so a trailing `>`, `]` or `,` is not handed to the parser as part
of the host (it rejects those outright, which would have lost the credential),
and the parsed host must look like a host, which keeps prose, email addresses,
git remotes and markdown links untouched.

One case is deliberately over-masked: a URL inside a JSON error body parses as
userinfo plus host `b`, textually identical to a password containing a quote.
Both readings cannot hold, so it errs toward masking — reasoning in
REVIEW_DECISIONS.md under url-masking-errs-toward-over-redaction-in-structured-text.

Verified: typecheck, lint, 1003 unit tests, 106 process tests, build,
git diff --check, and the probe sets re-run — 12 previously leaking or
false-positive cases now correct, every earlier regression case still holding,
idempotent, and linear at million-character scale.

Found while probing the two guards added last round: the host scan accepted
only ASCII, so `https://u:p@пример.example.com/x` truncated its candidate to
`https://u:p@`, which does not parse, and the credential was left visible. The
parser punycodes internationalized hosts rather than rejecting them, so they are
ordinary credentialed URLs and were the one host shape the new
parser-delegating design still got wrong.

The host test is now a predicate rather than a character class: anything above
ASCII counts, and expressing the ASCII half numerically also drops a
no-control-regex lint error that the range form introduced.

Ports, IPv6 literals with and without ports, percent-encoded hosts,
trailing-dot hosts, underscore hosts and long multi-label hosts were all checked
in the same pass and were already correct; tests pin the IDN, port and IPv6
cases.

Verified: typecheck, lint (0 errors), 1005 unit tests, 106 process tests, build,
git diff --check, and all four probe sets.

Twenty-eight inputs where the parser reported a credential and the output still
showed it. All of them traced to the same root cause: the host extent was being
decided by a character table narrower than what the parser accepts. That table
is gone.

Hosts can begin with characters the table rejected — `!example`, `,example`,
`{example` — and can be percent-encoded forms that decode to them, `%21example`
and `%2Cexample`. Rather than widening the table again, the candidate is now
offered to the parser at two extents: the conservative host, which stops at the
first character that certainly is not one and keeps `one.t,https://…` as two
URLs, and the structural end, which runs to the next `/?#` or space and accepts
everything else. Either verdict is safe, because only the userinfo is rewritten,
so the extent affects the decision and never the output. The plausibility guard
that used to sit on the parsed host is gone with the table.

An authority closed by the start of the next URL ended before its own host, so
`https://a:b@onehttps://safe/x` handed the parser a candidate with nothing to
validate. The host extent is now measured from the `@`, independent of where the
authority closed.

A space inside userinfo is the one case the scan cannot see, because in free text
a space almost always ends a URL and treating it otherwise would swallow prose
into every authority. WordPress Application Passwords contain spaces, and a
stored dashboardUrl reaching the debug redactor is exactly that shape, so a
whole-value fallback now runs when the scan finds nothing: if the entire value
parses as one credentialed URL it is masked as one. It runs only as a fallback,
so text holding several URLs still goes through the scan and every one is
masked, not just the first.

The fifth finding is pushed back in REVIEW_DECISIONS.md: a dropped character
immediately before the userinfo does not shift anything inside it, so masking in
place is precise and the placeholder would lose the scheme and host for no gain.

Verified: typecheck, lint (0 errors), 1008 unit tests, 106 process tests, build,
git diff --check, and all five probe sets — no leaks, free-text collateral
unchanged, idempotent, linear at million-character scale.

Round 11 review: the OpenAI-compatible stream keyed partial calls by index and
yielded nothing until the finish event, so the chat engine's cap could not
engage while a hostile endpoint opened fresh indices; the overflow guard also
sat after the finish block, so a final chunk carrying the overflowing delta
emitted the truncated call instead of throwing. maskUrlCredentials classified
the raw parameter key, so api%5Fkey passed through unmasked.

Two further findings (truncation splitting a spaced Application Password, soft
wrap escaping the untrusted-block prefix) are deferred to the structured-field
masking branch, documented in .mwpdev/reviews/REVIEW_DECISIONS.md.

CodeRabbit caught the encoded-key gap in the sibling path: maskUrlCredentials
decodes a parameter key before classifying it, sanitizeErrorMessage did not, so
an error echoing https://host/wp?api%5Fkey=SECRET printed the secret in full
while the same URL masked correctly on the display path. Both now share
isSensitiveParameterKey; the character classes stay separate because one scans
free prose and the other a whole URL.

Also asserts the F19 bounded-scan test returns the payload whole, so a scan
bound cannot be met by dropping content.

Sprint 1.1.1-R items 1-2 and Part C: the breaking
MAINWP_APP_PASSWORD/MAINWP_DASHBOARD_URL binding note and the security
fixes land in the Unreleased changelog section, and provider.ts now
states why the three streamed-tool-call bounds (8/2/1) are intentionally
distinct rather than derived from each other.